Skip to content

feat: add basic auth gateway to helm chart #4318

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mertcorumlu
Copy link

I added basic auth gateway support using ngnix.

This should address the problem in #2082 . The suggested solution cannot be used if you are not using ngnix controller. We are using a custom ingress controller, so it was essential to run a reverse proxy gateway for basic authentication.

I copied lots of components from the official Loki chart in https://github.com/grafana/loki/tree/main/production/helm/loki

I did not specify each single route in ngnix config file as it was done in Loki chart, just simply forward all traffix to Pyroscope service, except /health, which is required for pod readiness check.

@mertcorumlu mertcorumlu requested a review from a team as a code owner July 24, 2025 17:57
@CLAassistant
Copy link

CLAassistant commented Jul 24, 2025

CLA assistant check
All committers have signed the CLA.

Comment on lines +543 to +547
location ^~ {
set $api_prom_push_backend http://{{ include "pyroscope.fullname" .}}.{{ .Release.Namespace }}.svc.{{ .Values.gateway.clusterDomain }};
proxy_pass $api_prom_push_backend:{{- .Values.pyroscope.service.port -}}$request_uri;
proxy_http_version 1.1;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually needs to be broken down and should make sure to only forward the per service endpoints to the correct micro services. Forwarding all traffic to push endpoint, will not work for read traffic.

A good start is to look at the ingress resource here:

@simonswine simonswine self-assigned this Aug 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants